Improve performance of cloning AST nodes #19067
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
RobinMalfait
force-pushed
the
feat/canonicalize-candidates
branch
2 times, most recently
from
ccbd48d to
226ae47
Compare
RobinMalfait
force-pushed
the
feat/improve-cloning-performance
branch
from
3b92b93 to
2024aa8
Compare
thecrypticace
approved these changes
Oct 6, 2025
Our AST nodes are simple plain objects with a `kind` as the discriminant
instead of of an actual class instance. Since we're dealing with simple
plain objects, there is no need for a heavy handed `structuredClone` and
we can use this function instead.
Also added some benchmarks which you can run to see how much the
difference is. On my M1 Pro Max while using battery, these are the
results:
```
✓ Cloning AST nodes (2) 2462ms
name hz min max mean p75 p99 p995 p999 rme samples
· cloneAstNode() 5,769,838.55 0.0000 0.0467 0.0002 0.0002 0.0004 0.0004 0.0005 ±0.09% 2884920 fastest
· structuredClone() 222,598.38 0.0042 4.5958 0.0045 0.0045 0.0048 0.0049 0.0100 ±1.81% 111300
BENCH Summary
cloneAstNode() - ast.bench.ts > Cloning AST nodes
25.92x faster than structuredClone()
```
These are much faster compared to `structuredClone`. This is again because we're dealing with simple plain objects.
Turns out that if you keep the exact same structure instead of a spread,
it's even faster!
```
cloneAstNode() - src/ast.bench.ts > Cloning AST nodes
1.15x faster than cloneAstNode (with spread)
33.54x faster than structuredClone()
```
```
cloneCandidate - src/candidate.bench.ts > Candidate cloning
1.72x faster than cloneCandidate (spread)
74.03x faster than structuredClone
```
Strings are immutable, so the only thing we're cloning here is the array itself.
This just means that if you pass in an static candidate, that a static candidate comes out. Otherwise an static candidate results in the generic `Candidate` type which is too wide.
RobinMalfait
force-pushed
the
feat/improve-cloning-performance
branch
from
2024aa8 to
7e26cfc
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR improves the performance of when we need to clone some AST nodes. We have a few places where we clone
Candidate,Variantand CSSASTnodes.Right now we use
structuredClone, which works, but it is a generic solution. However, we do know the exact structure of these AST nodes, so we can write specialized clone functions that are much faster.Test plan
Ready for review, but should be merged after #19059